Conversation
|
This PR touches files which potentially affect the outcome of the tests of an exercise. This will cause all students' solutions to affected exercises to be re-tested. If this PR does not affect the result of the test (or, for example, adds an edge case that is not worth rerunning all tests for), please add the following to the merge-commit message which will stops student's tests from re-running. Please copy-paste to avoid typos. For more information, refer to the documentation. If you are unsure whether to add the message or not, please ping |
jagdish-15
left a comment
jagdish-15
left a comment
There was a problem hiding this comment.
Can you apply these small changes:
| import io.reactivex.disposables.Disposable; | ||
| import org.junit.jupiter.api.BeforeEach; |
There was a problem hiding this comment.
| import io.reactivex.disposables.Disposable; | |
| import org.junit.jupiter.api.BeforeEach; | |
| import io.reactivex.disposables.Disposable; | |
| import org.junit.jupiter.api.BeforeEach; |
| import org.junit.jupiter.api.Disabled; | ||
| import org.junit.jupiter.api.Test; | ||
| import org.junit.jupiter.api.DisplayName; |
There was a problem hiding this comment.
| import org.junit.jupiter.api.Disabled; | |
| import org.junit.jupiter.api.Test; | |
| import org.junit.jupiter.api.DisplayName; | |
| import org.junit.jupiter.api.Disabled; | |
| import org.junit.jupiter.api.DisplayName; | |
| import org.junit.jupiter.api.Test; |
jagdish-15
left a comment
jagdish-15
left a comment
There was a problem hiding this comment.
@kahgoh, I need your suggestion on a few of these:
| @Disabled("Remove to run test") | ||
| @Test | ||
| @DisplayName("Ability modifier for score 18 is 4") | ||
| public void testAbilityModifierForScore18Is4() { | ||
| assertThat(dndCharacter.modifier(18)).isEqualTo(4); | ||
| } |
There was a problem hiding this comment.
Can you please keep all of them the same as the descriptions in the canonical-data.json file?
And something to discuss, @kahgoh: only the tests up to this one are fully in sync with the canonical data. There are three more tests after this: random character is valid, random ability is within range, and each ability is only calculated once. But there are several extra tests in this file that are not present in the canonical data. Do you think we should keep them or make this file consistent with the canonical data?
There was a problem hiding this comment.
@kahgoh
This exercise doesn't have a canonical-data.json file, and I think we can go ahead with these descriptions. Do you have any nits for these?
There was a problem hiding this comment.
The same goes with this one!
@kahgoh Do you have any nits for the descriptions?
There was a problem hiding this comment.
@manjarekarsudip
All these should be in line with the canonical-data.json file
There was a problem hiding this comment.
There was a problem hiding this comment.
There was a problem hiding this comment.
There was a problem hiding this comment.
There was a problem hiding this comment.
There was a problem hiding this comment.
Again, this one seems to be a Java track-specific exercise, so it has no mention in the problem-specifications repo. I think we can go ahead with this one. Let's see what @kahgoh says about this
|
Is any other change required in any file? @jagdish-15 |
jagdish-15
left a comment
jagdish-15
left a comment
There was a problem hiding this comment.
For the exercises that don’t have a canonical-data.json file, I’ll need to give detailed changes for each of them. And for all the exercises that do have a canonical-data.json file, here’s the only change required:
There was a problem hiding this comment.
Can you replace this test and the one below it with each other, so the order is the same as that in the canonical data
jagdish-15
left a comment
jagdish-15
left a comment
There was a problem hiding this comment.
This is done for the error-handling exercise. Now the descriptions will be more consistent with each other and follow a similar (standardised) format like the canonical data
There was a problem hiding this comment.
| @DisplayName("Throws a general exception when requested") | |
| @DisplayName("Throws IllegalArgumentException") |
There was a problem hiding this comment.
| @DisplayName("Throws a checked exception (not a RuntimeException)") | |
| @DisplayName("Throws any checked exception") |
There was a problem hiding this comment.
| @DisplayName("Throws a checked exception with the provided detail message") | |
| @DisplayName("Throws any checked exception with provided detail message") |
There was a problem hiding this comment.
| @DisplayName("Throws an unchecked RuntimeException when requested") | |
| @DisplayName("Throws any unchecked exception") |
There was a problem hiding this comment.
| @DisplayName("Throws an unchecked RuntimeException with provided detail message") | |
| @DisplayName("Throws any unchecked exception with provided detail message") |
There was a problem hiding this comment.
| @DisplayName("Throws a custom checked exception") | |
| @DisplayName("Throws custom checked exception") |
There was a problem hiding this comment.
| @DisplayName("Throws a custom checked exception with provided message") | |
| @DisplayName("Throws custom checked exception with provided detail message") |
There was a problem hiding this comment.
| @DisplayName("Throws a custom unchecked exception") | |
| @DisplayName("Throws custom unchecked exception") |
There was a problem hiding this comment.
| @DisplayName("Throws a custom unchecked exception with provided message") | |
| @DisplayName("Throws custom unchecked exception with provided detail message") |
There was a problem hiding this comment.
| @DisplayName("Returns Optional<Integer> for valid input and empty Optional for invalid input") | |
| @DisplayName("Handles error by throwing Optional instance") |
jagdish-15
left a comment
jagdish-15
left a comment
There was a problem hiding this comment.
This one doesn’t have many changes. The few that are there are to make the descriptions a bit more compact
There was a problem hiding this comment.
| @DisplayName("Cannot play the same correct guess twice") | |
| @DisplayName("Cannot play the same guess twice") |
There was a problem hiding this comment.
| @DisplayName("Losing the game results in LOSS status and all parts present") | |
| @DisplayName("Losing the game results in LOSS status") |
There was a problem hiding this comment.
| @DisplayName("Winning the game reveals full word and marks WIN status") | |
| @DisplayName("Winning the game results in WIN status") |
There was a problem hiding this comment.
| @DisplayName("Initial game state is set correctly for a new word") | |
| @DisplayName("Initial game state is set correctly") |
jagdish-15
left a comment
jagdish-15
left a comment
There was a problem hiding this comment.
After these changes, you’ll notice some test descriptions start with Maze, some with Mazes, and some with Throws. Please move the two perfect checking tests whose descriptions start with Maze above the ones whose start with Mazes, so the groups are ordered cleanly.
There was a problem hiding this comment.
| public void theMazeHasOnlyOneEntranceOnTheLeftSide() { | |
| public void theMazeHasSingleEntranceOnTheLeftSide() { |
There was a problem hiding this comment.
| public void theMazeHasSingleExitOnTheRightSideOfTheMaze() { | |
| public void theMazeHasSingleExitOnTheRightSide() { |
There was a problem hiding this comment.
| @DisplayName("Generated maze has correct overall dimensions") | |
| @DisplayName("Maze has correct overall dimensions") |
There was a problem hiding this comment.
| @DisplayName("Maze contains only allowed characters") | |
| @DisplayName("Maze contains only valid characters") |
Let's keep the keyword the same in the test name and the description
There was a problem hiding this comment.
| @DisplayName("Consecutive maze generations produce different mazes") | |
| @DisplayName("Maze is different each time it is generated") |
Let's try to keep it consistent with the others
There was a problem hiding this comment.
| @DisplayName("Generated maze with seed is perfect (single path, no isolated cells)") | |
| @DisplayName("Maze with a seed is generated perfectly (single path, no isolated cells)") |
There was a problem hiding this comment.
| @DisplayName("Throws when rows less than minimum allowed") | |
| @DisplayName("Throws when rows are less than five") |
There was a problem hiding this comment.
| @DisplayName("Throws when columns less than minimum allowed") | |
| @DisplayName("Throws when columns are less than five") |
There was a problem hiding this comment.
| @DisplayName("Throws when rows exceed maximum allowed") | |
| @DisplayName("Throws when rows exceed hundred") |
There was a problem hiding this comment.
| @DisplayName("Throws when columns exceed maximum allowed") | |
| @DisplayName("Throws when columns exceed hundred") |
There was a problem hiding this comment.
| @DisplayName("Robot names are unique in a large sample") | |
| @DisplayName("Robot names are unique") |
There was a problem hiding this comment.
| @DisplayName("Robot has a valid name matching the pattern") | |
| @DisplayName("Robot has a valid name") |
There was a problem hiding this comment.
| @DisplayName("A new list is empty with size zero") | |
| @DisplayName("A new list is empty") |
There was a problem hiding this comment.
| @DisplayName("Create list from array sets correct size") | |
| @DisplayName("Create list from array") |
There was a problem hiding this comment.
| @DisplayName("Convert list to array returns correct element order") | |
| @DisplayName("Can return list as an array") |
exercises/practice/simple-linked-list/src/test/java/SimpleLinkedListTest.java
Outdated
Show resolved
Hide resolved
|
Just a general tip: instead of clicking |
|
Looks good to me, thanks! |
Welcome |
2 participants
Pull request
Description
Added @DisplayName annotations to test methods based on canonical data
Changes
Added DisplayName annotations to the following exercises:
Testing
All tests pass successfully (verified with ./gradlew test)
Related Issue
Addresses #2971 (DisplayName annotations initiative)
Note
Below 2 exercises are the ones that i observed have the changes done as per the requirement but the checkbox is bot ticked.
Reviewer Resources:
Track Policies